Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rt length to output ii #373

Open
wants to merge 13 commits into
base: add_rt_length_to_output
Choose a base branch
from

Conversation

mschwoer
Copy link
Collaborator

@mschwoer mschwoer commented Nov 13, 2024

First guess at what should be added in terms of raw file stats.

Also some renamings and refactoring of the pickle code.

cf. also #368

@@ -21,11 +21,11 @@
"name": "stderr",
"output_type": "stream",
"text": [
"0:00:00.000142 \u001b[32;20mPROGRESS: test\u001b[0m\n",
"0:00:00.000142 \u001B[32;20mPROGRESS: test\u001B[0m\n",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure what this was?

Copy link
Collaborator

@GeorgWa GeorgWa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think switching to time units as minutes is an important one. Everything else is good to go.

base_dict[f"{prefix}rt_limit_min"] = stats["rt_limit_min"]
base_dict[f"{prefix}rt_limit_max"] = stats["rt_limit_max"]

base_dict[f"{prefix}rt_duration_sec"] = stats["rt_duration_sec"]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually agree with Feng that we should convert the values to minutes. We can store them as minutes from the beginning.

verbosity="error",
)
# Log the full traceback
if self.path is None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the refactor 😄


self._config: Config = config

# deliberately not saving the actual raw data here to avoid the saved manager file being too large
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a comment why the dia_data goes back into PeptideCentric and doesn't stay here as one might expect?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it is

@@ -21,11 +21,11 @@
"name": "stderr",
"output_type": "stream",
"text": [
"0:00:00.000142 \u001b[32;20mPROGRESS: test\u001b[0m\n",
"0:00:00.000142 \u001B[32;20mPROGRESS: test\u001B[0m\n",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have colored logging. I don't know why but it looks like your machine likes a different ANSI escape than mine???

Copy link
Collaborator

@GeorgWa GeorgWa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants